feat(vue-query): Add mutationOptions#10036
Conversation
🦋 Changeset detectedLatest commit: 6abcbfb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds getter-aware typing utilities and a new type-level helper Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 6abcbfb
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/vue-query/src/__tests__/mutationOptions.test.ts`:
- Around line 1-11: The test imports onScopeDispose from vue-demi but never
mocks it, so casting it to MockedFunction and calling mockImplementation will
throw; add a vi.mock for 'vue-demi' that provides onScopeDispose as a
jest/vitest mock function (e.g., onScopeDispose: vi.fn()) and ensure the mock is
registered before the test uses the imported onScopeDispose (place the vi.mock
call at the top of the test file or adjust imports so the mock is applied prior
to importing onScopeDispose) so the test can call mockImplementation on
onScopeDispose safely.
🧹 Nitpick comments (2)
packages/vue-query/src/__tests__/mutationOptions.test.ts (2)
262-263: Test descriptions are misleading.These tests verify mutation state data (e.g.,
mutationStateArray[0]?.data), not "the number of fetching mutations." Consider updating the descriptions to accurately reflect what's being tested, e.g., "should return mutation states when used with useMutationState."
379-407: Test doesn't exercisemutationOptionsfunctionality.This test uses
useMutationdirectly without callingmutationOptions. It appears to test the disposal behavior ofuseMutation/useIsMutating, which may be better suited inuseMutation.test.tsoruseMutationState.test.ts.If the intent is to test
mutationOptionsintegration with disposal, consider usingmutationOptionsin the test setup:- const mutation = useMutation({ + const mutation = useMutation(mutationOptions({ mutationFn: (params: string) => sleep(0).then(() => params), - }) + }))
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10036 +/- ##
===========================================
- Coverage 45.84% 33.42% -12.43%
===========================================
Files 200 28 -172
Lines 8526 1071 -7455
Branches 1972 177 -1795
===========================================
- Hits 3909 358 -3551
+ Misses 4157 657 -3500
+ Partials 460 56 -404
🚀 New features to boost your workflow:
|
|
Why doesn't test coverage exclude the test files themselves? 🤔 query/packages/vue-query/vite.config.ts Line 25 in ed5e5fc |
shincurry
force-pushed
the
vue-mutation-options
branch
from
2210327 to
f05df98
Compare
There was a problem hiding this comment.
it should be enough here to reference the react docs, as the content is the same. For example, this is how queryOptions do it:
There was a problem hiding this comment.
you do need an entry in docs/config.json for the docs to show up though
| mutationFn: () => sleep(10).then(() => 5), | ||
| } as const | ||
|
|
||
| expect(mutationOptions(object)).toStrictEqual(object) |
There was a problem hiding this comment.
| expect(mutationOptions(object)).toStrictEqual(object) | |
| expect(mutationOptions(object)).toBe(object) |
| mutationFn: () => sleep(10).then(() => 5), | ||
| } as const | ||
|
|
||
| expect(mutationOptions(object)).toStrictEqual(object) |
There was a problem hiding this comment.
| expect(mutationOptions(object)).toStrictEqual(object) | |
| expect(mutationOptions(object)).toBe(object) |
|
@DamianOsipiuk could you have a look here please 🙏 |
2 participants
🎯 Changes
mutationOptionsDeepUnwrapRefOrGetter,MaybeRefDeepOrGettermutationOptions(Generated by Claude Opus 4.5, reviewed by @shincurry)mutationOptions(Copied from react-query)✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests
Chores